refactor: split bootstrap to smaller files for better readability#518
Conversation
WalkthroughThe changes refactor the bootstrap architecture by removing public middleware/controller/service interfaces and consolidating service initialization into a centralized app.context container and app.initServices method. A new app.setupRouter method replaces scattered route configuration, while services are wired into a public Services struct for organized dependency management. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant App as BootstrapApp
participant InitSvc as initServices()
participant DB as DatabaseService
participant LDAP as LdapService
participant Docker as DockerService
participant AC as AccessControlService
participant Auth as AuthService
participant OAuth as OAuthBrokerService
participant Router as setupRouter()
participant Engine as Gin Engine
Client->>App: NewBootstrapApp(config)
Client->>App: Run()
App->>InitSvc: call initServices()
InitSvc->>DB: Create & Init
DB-->>InitSvc: success
InitSvc->>LDAP: Create & Init
LDAP-->>InitSvc: (optional, tolerates failure)
InitSvc->>Docker: Create & Init
Docker-->>InitSvc: success
InitSvc->>AC: Create with Docker & Init
AC-->>InitSvc: success
InitSvc->>Auth: Create with composite config
Auth->>Docker: (dependency)
Auth->>LDAP: (dependency)
Auth->>DB: (dependency)
Auth-->>InitSvc: initialized
InitSvc->>OAuth: Create & Init
OAuth-->>InitSvc: success
InitSvc-->>App: Services struct
App->>Router: call setupRouter()
Router->>Engine: Configure middlewares
Router->>Engine: Mount controllers
Engine-->>Router: configured
Router-->>App: Gin Engine
App->>App: Start HTTP/Unix socket
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #518 +/- ##
==========================================
- Coverage 23.56% 23.37% -0.19%
==========================================
Files 36 38 +2
Lines 2245 2263 +18
==========================================
Hits 529 529
- Misses 1679 1697 +18
Partials 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/bootstrap/app_bootstrap.go (2)
98-132: Missing assignment:configuredProvidersnot stored inapp.context.The
configuredProvidersslice is built and validated but never assigned toapp.context.configuredProviders. This causes the issue flagged inroutes_bootstrap.gowhere an empty slice will be passed toContextController.Add the assignment before calling
setupRoutes():if len(configuredProviders) == 0 { return fmt.Errorf("no authentication providers configured") } + app.context.configuredProviders = configuredProviders + // Setup routes engine, err := app.setupRoutes()
226-238:gorm.G[model.Session]is undefined and will cause compilation failure.The code uses
gorm.G[T]syntax which is only available ingorm.io/genpackage, butgorm.io/genis not a dependency ingo.mod(onlygorm.io/gorm v1.31.1is present). Additionally, no generated query code or gen setup exists in the codebase. Standardgorm.io/gormdoes not exportG.Either add
gorm.io/genas a dependency and set up code generation, or replacegorm.G[model.Session](db)with standard GORM API likedb.Model(&model.Session{}).This affects lines 233 in app_bootstrap.go and multiple lines in auth_service.go.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/bootstrap/app_bootstrap.go(6 hunks)internal/bootstrap/routes_boostrap.go(1 hunks)internal/bootstrap/service_bootstrap.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/bootstrap/service_bootstrap.go (8)
internal/service/access_controls_service.go (2)
AccessControlsService(29-32)NewAccessControlsService(34-38)internal/service/auth_service.go (3)
AuthService(39-47)NewAuthService(49-57)AuthServiceConfig(28-37)internal/service/database_service.go (3)
DatabaseService(21-24)NewDatabaseService(26-30)DatabaseServiceConfig(17-19)internal/service/docker_service.go (2)
DockerService(14-18)NewDockerService(20-22)internal/service/ldap_service.go (3)
LdapService(24-28)NewLdapService(30-34)LdapServiceConfig(15-22)internal/service/oauth_broker_service.go (2)
OAuthBrokerService(21-24)NewOAuthBrokerService(26-31)internal/bootstrap/app_bootstrap.go (1)
BootstrapApp(23-36)internal/config/config.go (1)
SessionCookieName(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
internal/bootstrap/routes_boostrap.go (3)
12-22: LGTM!The Gin engine initialization with recovery middleware and trusted proxies configuration is correctly implemented with proper error handling.
24-55: LGTM!Middleware initialization follows a consistent pattern with proper error handling and wrapping.
71-104: LGTM!Controller instantiation and route setup follow a consistent pattern with proper dependency injection.
internal/bootstrap/service_bootstrap.go (3)
9-16: LGTM!The
Servicesstruct cleanly encapsulates all service dependencies with appropriate unexported fields for internal use.
50-68: LGTM!Docker and AccessControls service initialization follows proper dependency order with consistent error handling.
70-99: LGTM!Auth and OAuth broker service initialization is correctly implemented with proper configuration mapping and dependency injection.
internal/bootstrap/app_bootstrap.go (4)
23-36: LGTM!The
BootstrapAppstruct with nestedcontextfor runtime state is a clean approach for organizing configuration and runtime data.
44-97: LGTM!The initialization sequence is well-organized with proper error handling and trace logging for debugging.
134-170: LGTM!Server startup logic with unix socket support and cleanup routine initialization is correctly implemented.
172-224: LGTM!The heartbeat implementation with proper timeout, error handling, and resource cleanup is well-structured.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/bootstrap/app_bootstrap.go (1)
98-132: Critical: Missing assignment of configuredProviders to app.context.The
configuredProvidersslice is built locally (lines 99-119) but is never assigned toapp.context.configuredProvidersbeforeapp.setupRouter()is called at line 128. Sincerouter_boostrap.goline 59 readsapp.context.configuredProviders, the Context controller will receive an empty/nil providers slice, breaking authentication provider display.Apply this diff to assign the local variable to the context field:
if len(configuredProviders) == 0 { return fmt.Errorf("no authentication providers configured") } + app.context.configuredProviders = configuredProviders + // Setup router engine, err := app.setupRouter()
🧹 Nitpick comments (1)
internal/bootstrap/router_boostrap.go (1)
93-98: Consider a more explicit pattern for root router access.The
&engine.RouterGroupsyntax accesses the embedded RouterGroup field, which works but is somewhat unconventional. While this is valid Go and serves the purpose (Resources needs root-level routing for /resources, not /api/resources), consider usingengine.Group("")for clarity.resourcesController := controller.NewResourcesController(controller.ResourcesControllerConfig{ ResourcesDir: app.config.ResourcesDir, ResourcesDisabled: app.config.DisableResources, - }, &engine.RouterGroup) + }, engine.Group(""))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/bootstrap/app_bootstrap.go(6 hunks)internal/bootstrap/router_boostrap.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
internal/bootstrap/router_boostrap.go (11)
internal/bootstrap/app_bootstrap.go (1)
BootstrapApp(23-36)internal/middleware/context_middleware.go (2)
NewContextMiddleware(24-30)ContextMiddlewareConfig(14-16)internal/middleware/ui_middleware.go (1)
NewUIMiddleware(20-22)internal/middleware/zerolog_middleware.go (1)
NewZerologMiddleware(21-23)internal/controller/context_controller.go (3)
NewContextController(60-69)ContextControllerConfig(44-53)controller(71-75)internal/config/config.go (3)
Providers(178-180)CSRFCookieName(12-12)RedirectCookieName(13-13)internal/controller/oauth_controller.go (2)
NewOAuthController(36-43)OAuthControllerConfig(21-27)internal/controller/proxy_controller.go (2)
NewProxyController(31-38)ProxyControllerConfig(20-22)internal/controller/user_controller.go (2)
NewUserController(34-40)UserControllerConfig(24-26)internal/controller/resources_controller.go (2)
NewResourcesController(20-28)ResourcesControllerConfig(9-12)internal/controller/health_controller.go (2)
NewHealthController(9-13)router(5-7)
internal/bootstrap/app_bootstrap.go (5)
internal/config/config.go (6)
Config(17-46)User(77-81)OAuthServiceConfig(57-68)SessionCookieName(11-11)CSRFCookieName(12-12)RedirectCookieName(13-13)internal/controller/context_controller.go (1)
Provider(38-42)internal/bootstrap/service_bootstrap.go (1)
Services(9-16)internal/utils/app_utils.go (2)
GetOAuthProvidersConfig(129-202)GetCookieDomain(19-46)internal/utils/security_utils.go (1)
GenerateUUID(104-107)
🔇 Additional comments (9)
internal/bootstrap/router_boostrap.go (3)
12-22: LGTM! Clean router initialization.The router setup with recovery middleware and trusted proxies configuration is well-structured with appropriate error handling.
24-54: LGTM! Consistent middleware initialization pattern.All three middlewares (context, UI, zerolog) follow a consistent initialization pattern with proper error handling and descriptive error messages.
12-105: Excellent refactoring! Router setup is now centralized and readable.The extraction of router setup logic into a dedicated method improves code organization and maintainability. The consistent error handling and clear separation between middleware and controller setup makes the bootstrap flow easy to follow.
internal/bootstrap/app_bootstrap.go (6)
23-36: LGTM! Well-structured refactoring.The introduction of the internal
contextstruct to hold runtime state and theservicesfield for initialized services significantly improves code organization and makes the bootstrap lifecycle clearer.
44-88: LGTM! Configuration parsing correctly updated.All configuration values are properly parsed and assigned to
app.contextfields. The logging statements correctly reference the new context structure.
89-96: LGTM! Service initialization is clean and well-handled.The centralized service initialization via
app.initServices()with proper error wrapping is a good pattern.
127-169: LGTM! Clean startup sequence with proper error handling.The router setup, background routines (DB cleanup, heartbeat), and server startup logic are well-structured. The Unix socket handling with cleanup of existing files is a good practice.
172-224: LGTM! Heartbeat improvements are solid.The addition of a 10-second timeout to the HTTP client (lines 193-195) prevents indefinite blocking. The update to use
app.context.uuidcorrectly adapts to the refactored structure.
226-238: LGTM! Database cleanup routine is well-implemented.The 30-minute interval for cleaning expired sessions is reasonable, and the error handling ensures the routine continues even if individual cleanup attempts fail.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
internal/bootstrap/app_bootstrap.go (1)
89-127: Previous critical issue resolved:configuredProvidersis now properly assigned.The flow correctly:
- Initializes services (line 90)
- Builds the
configuredProvidersslice (lines 99-119)- Assigns to
app.context.configuredProviders(line 127)- Calls
setupRouter()(line 130)This ensures
app.context.configuredProvidersis available whenrouter_bootstrap.gocreates theContextController.
🧹 Nitpick comments (2)
internal/bootstrap/router_bootstrap.go (1)
12-22: Consider trimming whitespace when parsing trusted proxies.If the config contains spaces after commas (e.g.,
"proxy1, proxy2"), the second entry would include the leading space, potentially causing proxy matching failures.if len(app.config.TrustedProxies) > 0 { - err := engine.SetTrustedProxies(strings.Split(app.config.TrustedProxies, ",")) + proxies := strings.Split(app.config.TrustedProxies, ",") + for i := range proxies { + proxies[i] = strings.TrimSpace(proxies[i]) + } + err := engine.SetTrustedProxies(proxies)internal/bootstrap/app_bootstrap.go (1)
195-220: Good: HTTP client timeout prevents indefinite blocking.The 10-second timeout is appropriate for analytics heartbeats. One minor improvement would be using
deferfor body closure to ensure cleanup even if future code changes add early returns.res, err := client.Do(req) if err != nil { log.Error().Err(err).Msg("Failed to send heartbeat") continue } +defer res.Body.Close() -res.Body.Close() if res.StatusCode != 200 && res.StatusCode != 201 {Note: The current explicit
Close()is correct, butdeferis more resilient to future modifications.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/bootstrap/app_bootstrap.go(8 hunks)internal/bootstrap/router_bootstrap.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/bootstrap/app_bootstrap.go (5)
internal/config/config.go (6)
Config(17-46)User(77-81)OAuthServiceConfig(57-68)SessionCookieName(11-11)CSRFCookieName(12-12)RedirectCookieName(13-13)internal/controller/context_controller.go (1)
Provider(38-42)internal/bootstrap/service_bootstrap.go (1)
Services(9-16)internal/utils/app_utils.go (1)
GetOAuthProvidersConfig(129-202)internal/utils/security_utils.go (1)
GenerateUUID(104-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
internal/bootstrap/router_bootstrap.go (2)
24-54: Middleware initialization follows a clean, consistent pattern.Each middleware is instantiated, initialized with proper error handling, and attached to the engine. The error wrapping with
fmt.Errorfprovides good context for debugging.
56-102: Controller setup is well-structured.The API routes are correctly grouped under
/api, while the resources controller appropriately uses the root router group for static file serving. Each controller receives its required configuration and dependencies.internal/bootstrap/app_bootstrap.go (2)
23-36: Clean refactoring of state into nested context struct.The grouping of runtime-derived values (UUID, cookie names, providers) into
app.contextimproves organization and clearly separates configuration from computed state. The private Services field appropriately encapsulates service dependencies.
228-240: DB cleanup routine looks correct.The 30-minute interval for session cleanup is reasonable. The query correctly deletes sessions where the expiry timestamp is in the past.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.